Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

per_disc option for the ReplayGain plugin #3137

Merged
merged 10 commits into from
Jun 8, 2019

Conversation

samuelnilsson
Copy link
Contributor

@samuelnilsson samuelnilsson commented Feb 6, 2019

Should solve issue #293.

I'm unsure if these changes makes the rg_album_gain and rg_album_peak fields on the album level redundant. I've left it as is for now.

@samuelnilsson
Copy link
Contributor Author

It would be great if someone could test if the audiotools backend still works after these changes. I haven't got that backend to work on my system so haven't tested it myself.

@sampsyo
Copy link
Member

sampsyo commented Feb 6, 2019

Nice! This looks great already. If we can confirm that the audiotools backend still works, I think we should merge!

@samuelnilsson
Copy link
Contributor Author

Nice! This looks great already. If we can confirm that the audiotools backend still works, I think we should merge!

Okay, I tried to get the python audio tools backend to work again. However i noticed that I get the same error in the master branch. I don't know if I'm doing anything wrong or if this is a bug. I've used my changes without problems using the bs1770gain backend. Should I create a bug or am I doing something obviously wrong?

command: beet replaygain --force

system: Arch Linux

audiotools-config:
Default verbosity level : normal

extraction arguments
Format Readable Writable Default Quality
────── ──────── ──────── ───────────────
aiff yes yes
alac yes yes
au yes yes
flac yes yes 8
m4a yes yes 100
mp2 yes yes 192
mp3 yes yes 2
ogg yes yes 3
opus yes yes 10
tta yes yes
wav yes yes
wv yes yes standard

Format : %(track_number)2.2d - %(track_name)s.%(suffix)s
Default simultaneous jobs : 1
Add ReplayGain by default : no

ID3 arguments
ID3v2 tag version : ID3v2.3
ID3v2 digit padding : not padded ("1", "2", …)
ID3v1 tag version : ID3v1.1

CD lookup arguments
Use MusicBrainz service : no
Default MusicBrainz server : musicbrainz.org
Default MusicBrainz port : 80

      Use FreeDB service : no
   Default FreeDB server : us.freedb.org
     Default FreeDB port : 80

system arguments
Default CD-ROM device : /dev/cdrom
CD-ROM sample read offset : 0
CD-ROM sample write offset : 0
Filesystem text encoding : utf-8
Audio output : PulseAudio, ALSA, NULL

beets config:
plugins: replaygain

replaygain:
backend: audiotools

@samuelnilsson
Copy link
Contributor Author

Nice! This looks great already. If we can confirm that the audiotools backend still works, I think we should merge!

Okay, I tried to get the python audio tools backend to work again. However i noticed that I get the same error in the master branch. I don't know if I'm doing anything wrong or if this is a bug. I've used my changes without problems using the bs1770gain backend. Should I create a bug or am I doing something obviously wrong?

command: beet replaygain --force

system: Arch Linux

audiotools-config:
Default verbosity level : normal

extraction arguments
Format Readable Writable Default Quality
────── ──────── ──────── ───────────────
aiff yes yes
alac yes yes
au yes yes
flac yes yes 8
m4a yes yes 100
mp2 yes yes 192
mp3 yes yes 2
ogg yes yes 3
opus yes yes 10
tta yes yes
wav yes yes
wv yes yes standard

Format : %(track_number)2.2d - %(track_name)s.%(suffix)s
Default simultaneous jobs : 1
Add ReplayGain by default : no

ID3 arguments
ID3v2 tag version : ID3v2.3
ID3v2 digit padding : not padded ("1", "2", …)
ID3v1 tag version : ID3v1.1

CD lookup arguments
Use MusicBrainz service : no
Default MusicBrainz server : musicbrainz.org
Default MusicBrainz port : 80

      Use FreeDB service : no
   Default FreeDB server : us.freedb.org
     Default FreeDB port : 80

system arguments
Default CD-ROM device : /dev/cdrom
CD-ROM sample read offset : 0
CD-ROM sample write offset : 0
Filesystem text encoding : utf-8
Audio output : PulseAudio, ALSA, NULL

beets config:
plugins: replaygain

replaygain:
backend: audiotools

And the error:
replaygain: analyzing Pink Floyd - The Wall - In the Flesh?
Traceback (most recent call last):
File "/usr/bin/beet", line 11, in
load_entry_point('beets==1.5.0', 'console_scripts', 'beet')()
File "/usr/lib/python3.7/site-packages/beets-1.5.0-py3.7.egg/beets/ui/init.py", line 1267, in main
_raw_main(args)
File "/usr/lib/python3.7/site-packages/beets-1.5.0-py3.7.egg/beets/ui/init.py", line 1254, in _raw_main
subcommand.func(lib, suboptions, subargs)
File "/usr/lib/python3.7/site-packages/beets-1.5.0-py3.7.egg/beetsplug/replaygain.py", line 1052, in func
self.handle_track(item, write, force)
File "/usr/lib/python3.7/site-packages/beets-1.5.0-py3.7.egg/beetsplug/replaygain.py", line 1003, in handle_track
track_gains = backend_instance.compute_track_gain([item])
File "/usr/lib/python3.7/site-packages/beets-1.5.0-py3.7.egg/beetsplug/replaygain.py", line 752, in compute_track_gain
return [self._compute_track_gain(item) for item in items]
File "/usr/lib/python3.7/site-packages/beets-1.5.0-py3.7.egg/beetsplug/replaygain.py", line 752, in
return [self._compute_track_gain(item) for item in items]
File "/usr/lib/python3.7/site-packages/beets-1.5.0-py3.7.egg/beetsplug/replaygain.py", line 781, in _compute_track_gain
rg_track_gain, rg_track_peak = self._title_gain(rg, audiofile)
File "/usr/lib/python3.7/site-packages/beets-1.5.0-py3.7.egg/beetsplug/replaygain.py", line 764, in _title_gain
return rg.title_gain(audiofile.to_pcm())
File "/usr/lib/python3.7/site-packages/audiotools/mp3.py", line 217, in to_pcm
return MP3Decoder(self.filename)
TypeError: argument 1 must be str, not bytes

@sampsyo
Copy link
Member

sampsyo commented Jun 8, 2019

Aha! If this is broken on master, it's probably just a bug (or, more accurately, an API change in the library we're invoking). If this seems to work otherwise, I think it's ready to merge—and we can talk about fixing the error in a separate issue.

@samuelnilsson
Copy link
Contributor Author

Aha! If this is broken on master, it's probably just a bug (or, more accurately, an API change in the library we're invoking). If this seems to work otherwise, I think it's ready to merge—and we can talk about fixing the error in a separate issue.

Nice! I have used it frequently and it has worked well for me. I have created an issue at #3305 for the audiotools error.

@sampsyo
Copy link
Member

sampsyo commented Jun 8, 2019

Awesome! Thank you for your help with this!!! 🚀 ✨

@sampsyo sampsyo merged commit aea54e2 into beetbox:master Jun 8, 2019
@samuelnilsson samuelnilsson deleted the replaygain branch July 29, 2019 08:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants